refactor: establish SVG folder feature - #330
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1b67e4c26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| "dir": "src/svg", | ||
| "subdirs": true, | ||
| "feature": "SVG", |
There was a problem hiding this comment.
Make Canvas builds imply the new SVG feature
When a feature-gated build includes WebAPI.Canvas but omits the new WebAPI.SVG feature, moving these modules behind SVG makes Canvas fail because VideoFrame.res, Canvas.res, and CanvasRenderingContext2D.res reference SVGElement. I verified this by enabling every existing source feature except SVG: the parent compiles all 371 modules, while this commit reports that SVGElement cannot be found. Add a Canvas-to-SVG dependency edge so consumers do not need to discover this transitive requirement manually.
Useful? React with 👍 / 👎.
c1b67e4 to
69a53d2
Compare
Tracking issue: #342
Stack context
Part 20 of the Option 5 folder-feature stack.
Review this PR as the incremental diff against #329.
Summary
src/svgfolder on the unqualified internalSVGfeatureSVGElement,SVGGraphicsElement, andSVGLengthout of the legacy DOM folderWebAPI.SVG -> WebAPI.DOMNodes + WebAPI.Geometry + SVGfeatureTemporary state
WebAPI.DOMsource entry remains until its remaining interfaces have been assignedSVGElementand will acquire the SVG feature dependency in its later consolidation PRReview focus
Verification
node_modules/.bin/rescript cleannode_modules/.bin/rescript build --prod --features DOM,Event,DOMPlatform,Animation,CSSOM,Geometry,DOMNodes,SVGnpm run buildnpm testnpm run format:checkgit diff --check